-
Notifications
You must be signed in to change notification settings - Fork 14
feat: update error_details.proto to expose latest upstream fields #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @MathieuTricoire; unfortunately this isn't actually sufficient for you to access these fields as we provide wrappers around these types. See here. Is that something you feel like contributing?
@glbrntt Ah, got it — sorry, I rushed it a bit. I’ll update the wrappers as well! 👍 |
I've pushed the changes for the wrappers, I don't have much time today to have a deeper look and check if I'm missing something else or if my code blend in correctly, but feel free to take a first look and let me know what you think. I've added new initializers for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @MathieuTricoire. For some uninteresting reasons new API needs to be annotated with appropriate availability, I've left these as suggestions on the PR to save you from having to find them all.
Beyond that this looks good aside from one small nit re: Int/Int64
@glbrntt Thanks! I've added the availability annotations and fixed the Int/Int64 issue (I should have seen this one...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, Swift 6.0 CI is unhappy with the extra ,
/grpc-swift-protobuf/Sources/GRPCProtobuf/Errors/ErrorDetails+Types.swift:282:7: error: unexpected ',' separator
280 | subject: String,
281 | description: String,
282 | ) {
| `- error: unexpected ',' separator
283 | self.subject = subject
284 | self.violationDescription = description
/grpc-swift-protobuf/Sources/GRPCProtobuf/Errors/ErrorDetails+Types.swift:505:7: error: unexpected ',' separator
503 | field: String,
504 | description: String,
505 | ) {
| `- error: unexpected ',' separator
506 | self.field = field
507 | self.violationDescription = description
Oops sorry I'll fix that |
### What changes were proposed in this pull request? This PR aims to upgrade `grpc-swift-protobuf` to 2.1.1. ### Why are the changes needed? To bring the latest improvements and bug fixes. - https://github.com/grpc/grpc-swift-protobuf/releases/tag/2.1.1 - https://github.com/grpc/grpc-swift-protobuf/releases/tag/2.1.0 - grpc/grpc-swift-protobuf#77 - grpc/grpc-swift-protobuf#82 - grpc/grpc-swift-protobuf#81 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass the CIs. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #217 from dongjoon-hyun/SPARK-53370. Authored-by: Dongjoon Hyun <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
Motivation
The upstream Google proto file
error_details.proto
has been updated (see: https://github.com/googleapis/googleapis/blob/2e1c38ddaa402303b9c1381f5f9ba5a9ced4a51c/google/rpc/error_details.proto):QuotaFailure
message:api_service
,quota_metric
,quota_id
,quota_dimensions
,quota_value
andfuture_quota_value
BadRequest.FieldViolation
message:reason
andlocalized_message
Modifications
Update the google proto files and generates the stubs using the provided scripts
dev/protos/fetch.sh
anddev/protos/generate.sh
.Result
Library users can now access the newly added fields in
error_details.proto
.